Allow users to opt-out of changing instance name tags #3
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request introduces a new feature allowing users to opt-out of changing instance name tags in Auto Scaling Groups (ASGs). The changes primarily affect the README.md and lambda/autoscale/autoscale.py files.
- Added new
asg:update_instance_nametag inlambda/autoscale/autoscale.pyto control instance name tag updates - Updated
README.mdwith documentation on how to use the new opt-out feature - Modified
fetch_tag_metadataandprocess_messagefunctions inlambda/autoscale/autoscale.pyto implement the opt-out logic - CHANGELOG.md should be updated to reflect this new feature
- Consider adding tests for the new opt-out functionality
2 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
| tags = autoscaling.describe_tags( | ||
| Filters=[ | ||
| {'Name': 'auto-scaling-group','Values': [asg_name]}, | ||
| {'Name': 'key','Values': [HOSTNAME_TAG_NAME]} | ||
| {'Name': 'key','Values': [HOSTNAME_TAG_NAME, UPDATE_INSTANCE_TAG_NAME]} | ||
| ], | ||
| MaxRecords=1 | ||
| )['Tags'][0]['Value'] |
There was a problem hiding this comment.
logic: The 'Tags' key is accessed without checking if it exists or if it's empty. This could lead to an IndexError if no tags are found.
| tag_values = { | ||
| tag['Key']: tag['Value'] for tag in tags | ||
| } |
There was a problem hiding this comment.
logic: This dictionary comprehension assumes that 'tags' is a list of dictionaries with 'Key' and 'Value' keys. However, 'tags' is actually the value of the first tag found. This will likely cause a runtime error.
| if asg_tag_metadata.get(UPDATE_INSTANCE_TAG_NAME, "true") == "true": | ||
| update_name_tag(instance_id, hostname) |
There was a problem hiding this comment.
style: The default value 'true' is a string, not a boolean. Consider using a boolean default value instead.
| @@ -12,6 +12,7 @@ | |||
| route53 = boto3.client('route53') | |||
|
|
|||
| HOSTNAME_TAG_NAME = "asg:hostname_pattern" | |||
| UPDATE_INSTANCE_TAG_NAME = "asg:update_instance_name" | |||
There was a problem hiding this comment.
style: Consider adding a comment explaining the purpose and expected values of UPDATE_INSTANCE_TAG_NAME for better code documentation.
I was very surprised when my instance names changed after adding this. I much prefer their original names, so I added a way to opt-out of of them being change. 😄